-
Notifications
You must be signed in to change notification settings - Fork 804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(sdk-metrics): collect metrics when periodic exporting metric reader flushes #3517
Conversation
e22a9dc
to
4e1565b
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3517 +/- ##
=======================================
Coverage 93.80% 93.80%
=======================================
Files 249 249
Lines 7637 7640 +3
Branches 1589 1589
=======================================
+ Hits 7164 7167 +3
Misses 473 473
|
4e1565b
to
0d7ce4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for adding this. 🙂
I remember we agreed to remove it from the original PR about a year ago as it was not part of the spec, glad to see that it's back. 🙂
Does this also cause metrics to be collected on shutdown? |
@aabmass No. This PR addresses the change in open-telemetry/opentelemetry-specification#3084. To include effects of force flush in sdk shutdown, I've submitted open-telemetry/opentelemetry-specification#3085 and I can submit another PR to update it in JS SDK. |
Sounds good to me, thanks for following up! |
Which problem is this PR solving?
Fixes #3278
Related spec issue: open-telemetry/opentelemetry-specification#2983
Short description of the changes
Type of change
How Has This Been Tested?
PeriodicExportingMetricReader.forceFlush
collects and exports metrics.Checklist: